feat(eval): batch-evaluation simulate — each example owns its invoker - #2032
feat(eval): batch-evaluation simulate — each example owns its invoker#2032jariy17 wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## refactor #2032 +/- ##
============================================
- Coverage 97.13% 97.10% -0.04%
============================================
Files 381 388 +7
Lines 22786 23158 +372
============================================
+ Hits 22134 22487 +353
- Misses 652 671 +19 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
7ac54d4 to
6a2915e
Compare
6a2915e to
b747257
Compare
Hweinstock
left a comment
There was a problem hiding this comment.
didn't get to all the examples work yet, but had a few small comments and a question on how we can simplify testing, because it feels pretty gnarly rn.
| @@ -0,0 +1,33 @@ | |||
| import { InputValidationError } from "../errors"; | |||
There was a problem hiding this comment.
should we keep this inside evals until there is an opportunity to re-use it? I feel like the io directory should be reserved for shared abstractions.
There was a problem hiding this comment.
Yeah, I was thinking the invoke handler could use this but lets leave it in the simulate handler for now.
| try { | ||
| parsed = JSON.parse(template); | ||
| } catch { | ||
| throw new InputValidationError(`--${flagName} must be valid JSON`); |
There was a problem hiding this comment.
should we wire the cause here?
| qualifier?: string; | ||
| payloadTemplate: string; // e.g. {"prompt":"{input}"} — {input} is the example's turn input | ||
| headers?: [string, string][]; | ||
| bearerToken?: string; |
There was a problem hiding this comment.
q: is there a reason bearerToken is treated different from other headers?
There was a problem hiding this comment.
It's the discriminator that selects the auth path, not an application header. Its presence routes the invoke to CUSTOM_JWT path (src/core/invokeRuntime.ts:73)
| flag("dataset-version", "dataset version (with a dataset id)", z.string().optional()), | ||
| flag("evaluator", "evaluator id(s) to apply", z.array(z.string()).optional()), | ||
| flag("name", "batch evaluation name (unique in the account)", z.string().optional()), | ||
| flag("description", "optional description", z.string().optional()), |
There was a problem hiding this comment.
what exactly is this describing? the simulation itself?
There was a problem hiding this comment.
This should be batch-evaluation description. I'll rename this batch-eval-desc, same with name too.
| throw new InputValidationError("required option '--name <name>' not specified"); | ||
|
|
||
| // Ctrl-C aborts the run (invokes, the ingestion wait, the dataset download). | ||
| const controller = new AbortController(); |
There was a problem hiding this comment.
ahhh, theres a shared abstraction for this, but looks like it hasn't been merged yet :(
maybe we can swap it over as a follow-up once its merged.
| const groundTruth = await example.run(ctx); | ||
| return { exampleId: example.exampleId, sessionId, groundTruth }; | ||
| } catch (error) { | ||
| this.logger.debug( |
There was a problem hiding this comment.
instead of logging and rethrowing, is there a way to enrich the error thrown to avoid noise?
|
|
||
| // AgentCore emits spans ~30s-3min after invoke; grade too early and it reads an empty | ||
| // log group and fails every session. Disabled via SIMULATE_INGESTION_WAIT_MS=0 (tests). | ||
| const waitMs = Number(process.env.SIMULATE_INGESTION_WAIT_MS ?? 180_000); |
There was a problem hiding this comment.
is there anything we can poll on instead of a static wait time?
There was a problem hiding this comment.
I didn't poll right away because some traces might still be arriving, and I didn't want us to end up with incomplete session data.
|
|
||
| // A fake AWS layer: control resolves the runtime, data answers each invoke. Records every | ||
| // payload it was asked to send, and per `opts` can fail or delay specific invokes. | ||
| function fakeClients(opts: { fail?: (payload: string) => boolean; delayMs?: number } = {}): { |
There was a problem hiding this comment.
this feels like a really complex testing setup. Is there a simpler way? I'm wondering if there's a structural change we could make to simplify here.
| let parsed: unknown; | ||
| try { | ||
| parsed = JSON.parse(trimmed); | ||
| } catch { |
There was a problem hiding this comment.
should we wire the cause here?
| import type { DatasetSchemaType } from "@aws-sdk/client-bedrock-agentcore-control"; | ||
| import type { InlineGroundTruth } from "@aws-sdk/client-bedrock-agentcore"; | ||
|
|
||
| // A record, not a bare string, so a future tool-branching type can widen it by a field. |
There was a problem hiding this comment.
i feel like the code explains this comment.
- move renderJsonTemplate out of shared src/io into core/eval/invokeDataset - invokeRuntime: raw TypeError/Error -> InputValidationError/RuntimeInvokeResponseError - wire error causes in template + dataset JSON parse - invokeDataset: enrich per-example invoke failure instead of log+rethrow - simulate: bubble Ctrl-C cancellation (telemetry) instead of quiet return; clarify --description help; TODO(#1986) shared abort helper - drop type-guaranteed 'no leak' test; keep AbortSignal wiring in composition test - trim stale/redundant comments (runtime.tsx, invokeRuntime DTO note, TurnResult)
What
Adds
batch-evaluation simulateto replay a dataset against a runtime and grade the resulting sessions.The invoker seam
A dataset example knows what to send and how to turn the responses into ground truth, but not how to reach the runtime. Its
RunContextowns that runtime-specific invocation:invokeDatasetresolves the runtime once, then builds a distinct invoker for each client-generated session and passes it to that example:The example decides how many times to call
invokeOnceand in what order. Its invoker decides how each call reaches the runtime for that session.Layout
Plus
src/core/invokeRuntime.ts, which extracts runtime invocation fromruntime.tsxfor reuse by bothRuntimeClientandinvokeDataset, andsrc/handlers/eval/batch-evaluation/simulate/index.tsx, which composesinvokeDatasetwithstartBatchEvaluation.Testing
bun run typecheckpasses.bun test: 1544 pass, 0 fail.invokeDataset.test.tsdrives the realEvalClient.invokeDatasetover a fake AWS layer with golden fixtures. Its snapshot covers created sessions and inline ground truth across every supported variation while exercising the loader, example classes, concurrency pool, template rendering, and runtime invocation.simulate.test.tsxsnapshots the handler's wrappedsessionMetadata.